Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix/feat: flyway V5 DML이 적용되지 않는 문제 해결 + 테스트 컨테이너 적용 #729

Merged
merged 21 commits into from
Oct 17, 2024

Conversation

hyeon0208
Copy link
Contributor

@hyeon0208 hyeon0208 commented Oct 15, 2024

🚩 연관 이슈

close #712

자세한 내용들은 아래 포스팅했습니다
https://hstory0208.tistory.com/entry/%EA%B0%9C%EB%B0%9C-%EC%9A%B4%EC%98%81-%ED%99%98%EA%B2%BD%EA%B3%BC-%EB%B9%84%EC%8A%B7%ED%95%9C-%EB%A1%9C%EC%BB%AC-%ED%99%98%EA%B2%BD-%EA%B5%AC%EC%B6%95%ED%95%98%EA%B8%B0-feat-TestContainer


📝 작업 내용

flyway V5 DML이 적용되지 않는 문제 해결

flyway가 v3의 주석은 읽지만 v5의 주석을 읽지 못하는 문제가 있었어요
때때로 이런 문제가 있다고 하는것 같은데 근복적으로 주석이 문제여서 전체적으로 주석을 제거했어요

v3, v5 파일의 주석을 제거하면서 flyway 히스토리 내역을 지우고 다시 마이그레이션을 진행해야할 것 같아
같이 적용할 김에 v2 파일에 체크제약 조건 drop 구문을 추가해 로컬에서 실행할 때 체크제약 조건이 없어 제거가안되는 문제 또한 같이 해결하고자 했습니다.


로컬 환경과 dev, prod 환경의 DB 동일하게 세팅

flyway를 도입하게 되면서 dml 들이 mysql 종속적이여서 h2에선 실행이 안되는 등 많은 문제가 있었죠 😭
이 문제 발생을 줄이고자 동일한 mysql db를 사용하도록 수정했습니다.
각자 로컬에서 mysql 세팅을 하는 것은 귀찮을 테니 docker compose를 사용한 방식을 적용했는데요
이 또한 애플리케이션 실행 전마다 compose up하는게 귀찮잖아요 ..?
gradle-docker 플러그인을 적용해서 애플리케이션 실행 할 때 자동으로 compose up을 하고 실행할 수 있도록 적용해봤어요.
단 아래와 같은 간단한 설정이 필요합니다.


테스트 컨테이너 도입

위와 같은 문제로 현재는 DB 종속적인 테스트는 없어서 문제가 없지만
똑같은 지옥을 겪는 것을 방지하고자 환경을 통일하면서 테스트도 통일할 수 있도록 테스트 컨테이너 추가했어요


DatabaseCleaner 리팩토링

테스트 컨테이너와 @DataJpaTest를 같이 사용할 때 롤백이 정상적으로 이뤄지지 않는 경우들이 있어
테스트의 멱등성이 보장되지 않아 BaseRepositoryTest에도 DataBaseCleaner를 사용한 클린업작업을 추가했어요
추가로 DataBaseCleaner의 코드를 테이블 리스트를 찾아 반복문을 돌면서 TUNCATE하도록 리팩토링 했습니다.


🏞️ 스크린샷 (선택)


🗣️ 리뷰 요구사항 (선택)

Copy link

github-actions bot commented Oct 15, 2024

Test Results

156 tests   156 ✅  5s ⏱️
 45 suites    0 💤
 45 files      0 ❌

Results for commit 7fc7457.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Oct 15, 2024

📝 Test Coverage Report

Overall Project 79.95%

There is no coverage information present for the Files changed

Copy link
Contributor

@eun-byeol eun-byeol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

고생하셨어요 카키!! 블로그 보니 이해가 잘 됐어요.
덕분에 디비 환경 달라서 발생하는 문제는 없겠네요~
질문을 몇개 남겼는데, 간단하게만 주셔도 괜찮아요! 체크 못할 것 같아 RC 남깁니다!!

+) 참고사항

  1. 테스트 돌리는데 약 30초가 걸린다는 점..
  2. 테스트 성공했는데, 끝나지 않고 계속 돈다는 점
  • 카키에게 듣기로, 커버리지 체크 & 리포팅용으로 사용하고 있는 jacoco 라이브러리 때문이라고 하던데, 없애는 건 좀 그렇겠죠...?

init:
mode: always

ddl-auto: create-drop
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[제안] 물론 테이블 일회성으로 사용하지만 로컬에서도 validate으로 검증할 수 있으면 더 좋을 것 같아요!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

로컬에서는 docker gradle 플러그인을 사용해
애플리케이션 실행시 docker-compose up을 실행한 후 애플리케이션 실행을 자동화했는데
애플리케이션 종료 시에 항상 직접 컨테이너를 닫아주지 않는 한 컨테이너가 종료되지 않아서
데이터가 누적되는 문제 땜에 create-drop으로 바꾸긴했어요

저희는 로컬에서 사용할 db는 애플리케이션 실행마다 초기화되길 원하기 때문에
이 부분도 테스트 컨테이너를 도입하면 편할꺼 같긴하네요
다들 어떻게 생각하시나요 ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

app db도 테스트 컨테이너 도입해서 validate으로 바꾸는게 좋아요.
validate이어야 빠르게 오류 상황 잡고, 배포 후 오류 가능성을 줄일 수 있을 것 같습니다!

Comment on lines +1 to +3
ALTER TABLE notification DROP CONSTRAINT notification_chk_1;

ALTER TABLE notification ADD CONSTRAINT notification_chk_1 CHECK (`type` IN ('DEPARTURE_REMINDER', 'ENTRY', 'NUDGE', 'MEMBER_DELETION', 'ETA_NOTICE'));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[질문] drop 후 다시 add 해주는 이유가 궁금해요!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

v1 스키마가 실행될 때 type에 대한 제약조건 notification_chk_1이 생성되는데
이 제약조건을 지우고 새로운 타입 체크가 추가된 notification_chk_1를 추가해주기 위해서 �drop을 추가했습니다 ~!

기존에 문제가 되지 않았던 이유는 dev, prod db에 접근해서 제약조건을 임의로 제거했기 때문이였어요

.executeUpdate();
@Transactional
public void cleanUp() {
entityManager.createNativeQuery(FOREIGN_KEY_CHECK_OFF).executeUpdate();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

오.. 훨씬 간단해졌네요!👍


entityManager.createNativeQuery("ALTER TABLE api_call ALTER COLUMN id RESTART WITH 1")
.executeUpdate();
@Transactional
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[질문] 트랜잭션 어노테이션의 역할이 무엇인가요?
없애면 TransactionRequiredException 에러가 나는데 정확한 이유를 잘 모르겠어요ㅎㅎ

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

영속성 컨텍스트가 트랜잭션 범위 내에서 동작해야하기 떄문입니다 ~!

@woowacourse-teams woowacourse-teams deleted a comment from hyeon0208 Oct 16, 2024
@woowacourse-teams woowacourse-teams deleted a comment from hyeon0208 Oct 16, 2024
@hyeon0208
Copy link
Contributor Author

release V 2.0.0

이야기 한데로 로컬 애플리케이션의 DB는 애플리케이션을 재시작해도 데이터 유지되도록 validate 옵션으로 변경했고
flyway로 dev, prod 마이그레이션 시 V2 파일부터 적용하도록 baseline-version 옵션을 2로 지정했어요

그리고 콜리 컴퓨터에서 테스트 컨테이너로 테스트 실행 시 너무 긴 시간이 소요된다는 점으로 h2를 사용한 테스트로 롤백해
테스트에선 flyway의 스크립트가 실행되지 않도록 수정했어요

Copy link
Contributor

@mzeong mzeong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

드디어 로컬에서도 mysql을 사용하게 되었네요 😄😄😄
블로그로 내용을 잘 공유해주어 고맙습니다 approve 할게요~

Copy link
Contributor

@eun-byeol eun-byeol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

고생하셨습니다!! 덕분에 로컬 app에서도 운영과 동일한 환경에서 실행할 수 있겠네요~👍

Comment on lines +13 to +14
env:
TZ: 'Asia/Seoul'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[질문]
사용을 안 하고 있는것 같은데 맞을까요? 제거부탁드려요~

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

모든 job들에 타임존 설정이 적용되고 있어서
시간 계산 로직으로 인한 사이드 이펙트 방지로 들어가면 좋을꺼같아요 !

Copy link
Contributor

@coli-geonwoo coli-geonwoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dev 서버 회생을 위해 우선 approve가 필요하다 하여 approve 후 코드 검토해보겠습니다!!

@hyeon0208 hyeon0208 merged commit abbabf5 into develop Oct 17, 2024
1 check passed
@hyeon0208 hyeon0208 deleted the feature/712 branch October 17, 2024 08:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

fix: flyway V5 DML이 적용되지 않는 문제 해결
4 participants